Skip to content

feat: use logger instead of console where possible - #14470

Merged
ArmandPhilippot merged 6 commits into
v7.3from
feat/runtime-logger
Sep 2, 2026
Merged

feat: use logger instead of console where possible#14470
ArmandPhilippot merged 6 commits into
v7.3from
feat/runtime-logger

Conversation

@florian-lefebvre

@florian-lefebvre florian-lefebvre commented Aug 26, 2026

Copy link
Copy Markdown
Member

Description (required)

Updates API references

References

  • For Astro version 7.3.0

@florian-lefebvre florian-lefebvre mentioned this pull request Aug 26, 2026
Merged
@astrobot-houston

astrobot-houston commented Aug 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

File Note
en/reference/cache-provider-reference.mdx Source changed, localizations will be marked as outdated.
en/reference/image-service-reference.mdx Source changed, localizations will be marked as outdated.
Warnings reference
Icon Description
🔄️ The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

@github-actions

github-actions Bot commented Aug 26, 2026 •
edited
Loading

Copy link
Copy Markdown

Preview deployment

✅ Deployment complete!

@florian-lefebvre
florian-lefebvre marked this pull request as ready for review August 26, 2026 08:23
ArmandPhilippot
ArmandPhilippot reviewed Aug 26, 2026
View reviewed changes

@ArmandPhilippot ArmandPhilippot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Florian! Sorry this is only a partial review for now as I have to go. But, I suspect we want a similar change on the transform() API.

My thinking is that onRequest() and transform() remain the main topic. The logger description is maybe "too visible" and can distract from the main purpose of the section. The goal is not to log, even though this is a nice addition. 😄

Comment thread src/content/docs/en/reference/cache-provider-reference.mdx Outdated
@ArmandPhilippot ArmandPhilippot added merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) minor-release For the next minor release; in the milestone, "merge queue" = approved for merging on release day. labels Aug 26, 2026
@ArmandPhilippot ArmandPhilippot added this to the 7.3 milestone Aug 26, 2026
ArmandPhilippot
ArmandPhilippot reviewed Aug 26, 2026
View reviewed changes

@ArmandPhilippot ArmandPhilippot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review part 2, see my comment. This is a similar feedback as the one I gave on onRequest(), but as TL/DR: I think it's even trickier than the previous one because of the four distinct parameters.

Not right now, but I’ll try to take the time to think about how we can improve the API references. I think highlighting optional properties is helpful (and this was already on my TODO list)... but maybe we should also have a way to highlight additional arguments. They do not always fit well into the existing description / this requires a more extensive rewrite to include them. 🤔

Comment thread src/content/docs/en/reference/image-service-reference.mdx Outdated
@florian-lefebvre

florian-lefebvre commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

Always happy to find weird cases 😂! I think your suggestions made sense, done them locally

ArmandPhilippot
ArmandPhilippot approved these changes Aug 27, 2026
View reviewed changes

@ArmandPhilippot ArmandPhilippot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM! 🙌🏽

ArmandPhilippot
ArmandPhilippot approved these changes Aug 28, 2026
View reviewed changes

@ArmandPhilippot ArmandPhilippot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a suggestion regarding parseURL() because I think there is an issue with the flow. Otherwise, LGTM!

Comment thread src/content/docs/en/reference/image-service-reference.mdx Outdated
yanthomasdev
yanthomasdev reviewed Sep 1, 2026
View reviewed changes
Comment thread src/content/docs/en/reference/cache-provider-reference.mdx Outdated
yanthomasdev
yanthomasdev approved these changes Sep 1, 2026
View reviewed changes

@yanthomasdev yanthomasdev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work @florian-lefebvre, a small nit but then LGTM

@florian-lefebvre @yanthomasdev
Update src/content/docs/en/reference/cache-provider-reference.mdx
1844bc9 
Co-authored-by: Yan <61414485+yanthomasdev@users.noreply.github.com>
@ArmandPhilippot
ArmandPhilippot changed the base branch from main to v7.3 September 2, 2026 13:00
@ArmandPhilippot

ArmandPhilippot commented Sep 2, 2026

Copy link
Copy Markdown
Member

Thanks again, I updated the branch to merge this to an umbrella PR for the 7.3 release.

@ArmandPhilippot
ArmandPhilippot merged commit 39529fd into v7.3 Sep 2, 2026
9 checks passed
@ArmandPhilippot
ArmandPhilippot deleted the feat/runtime-logger branch September 2, 2026 13:02
@ArmandPhilippot ArmandPhilippot mentioned this pull request Sep 2, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ArmandPhilippot ArmandPhilippot ArmandPhilippot approved these changes
@yanthomasdev yanthomasdev yanthomasdev approved these changes

Assignees

No one assigned

Labels

merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) minor-release For the next minor release; in the milestone, "merge queue" = approved for merging on release day.

Projects

None yet

Milestone

7.3

Development

Successfully merging this pull request may close these issues.

4 participants

@florian-lefebvre @astrobot-houston @ArmandPhilippot @yanthomasdev